Skip to content

Fix perf regression in rustdoc::bare_urls #84034

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 9, 2021
Merged

Fix perf regression in rustdoc::bare_urls #84034

merged 1 commit into from
Apr 9, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Apr 9, 2021 •
edited
Loading

This regressed in #81764. After that PR, rustdoc compiled the regex for every single item in the crate: https://perf.rust-lang.org/compare.html?start=125505306744a0a5bb01d62337260a95d9ff8d57&end=2e495d2e845cf27740e3665f718acfd3aa17253e&stat=instructions%3Au

This would have been caught by clippy::declare_interior_mutable_const (cc #77983).

@jyn514
Fix perf regression in non-autolinks
8c7b7dd 
Before, this was compiling the regex on every single item in the crate.

This would have been caught by `clippy::declare_interior_mutable_const`.
@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. I-compiletime Issue: Problems and improvements with respect to compile times. labels Apr 9, 2021
@rust-highfive
Copy link
Contributor

r? @ollie27

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 9, 2021
@jyn514
Copy link
Member Author

jyn514 commented Apr 9, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 9, 2021
@bors
Copy link
Collaborator

bors commented Apr 9, 2021

⌛ Trying commit 8c7b7dd with merge 47a47f0f8ba775de36a751857867bfbea8310770...

@Mark-Simulacrum
Copy link
Member

r=me presuming perf looks good

@bors
Copy link
Collaborator

bors commented Apr 9, 2021

☀️ Try build successful - checks-actions
Build commit: 47a47f0f8ba775de36a751857867bfbea8310770 (47a47f0f8ba775de36a751857867bfbea8310770)

@rust-timer
Copy link
Collaborator

Queued 47a47f0f8ba775de36a751857867bfbea8310770 with parent bc66b92, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (47a47f0f8ba775de36a751857867bfbea8310770): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 9, 2021
@jyn514
Copy link
Member Author

jyn514 commented Apr 9, 2021

Yup, that was it.

@bors r=Mark-Simulacrum rollup=never

@bors
Copy link
Collaborator

bors commented Apr 9, 2021

📌 Commit 8c7b7dd has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 9, 2021
@bors
Copy link
Collaborator

bors commented Apr 9, 2021

⌛ Testing commit 8c7b7dd with merge 8513e78...

@bors
Copy link
Collaborator

bors commented Apr 9, 2021

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 8513e78 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 9, 2021
@bors bors merged commit 8513e78 into rust-lang:master Apr 9, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 9, 2021
@jyn514 jyn514 deleted the regex-in-loop branch April 9, 2021 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ollie27 ollie27

Labels
I-compiletime Issue: Problems and improvements with respect to compile times. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.53.0
Development

Successfully merging this pull request may close these issues.
7 participants
@jyn514 @rust-highfive @rust-timer @bors @Mark-Simulacrum @ollie27 @rustbot